Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define Sec-Fetch-Frame-Ancestors. #89

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

mikewest
Copy link
Member

This patch aims to formalize the discussion from
#56, defining a Sec-Fetch-Frame-Ancestors header that supplements Sec-Fetch-Site's exposure of the relationship between a request's initiator and the request's target with additional context about the same-siteness of context within which the request was made. This should support developers' understanding of the viability of various SameSite cookie settings on the one hand, and the ways in which their resources are partitioned on the other.

This patch aims to formalize the discussion from
#56, defining a `Sec-Fetch-Frame-Ancestors`
header that supplements `Sec-Fetch-Site`'s exposure of the relationship
between a request's initiator and the request's target with additional
context about the same-siteness of context within which the request was
made. This should support developers' understanding of the viability of
various `SameSite` cookie settings on the one hand, and the ways in
which their resources are partitioned on the other.
@mikewest mikewest mentioned this pull request Apr 30, 2024
@annevk
Copy link
Member

annevk commented Apr 30, 2024

I wonder if we should allow for two separate pieces of information:

  • What's the relationship with the top-level document: same-origin, same-site, or cross-site.
  • What's the relationship with all ancestors: same-origin, same-site, or cross-site.

The platform (including our shared understanding of the envisioned platform) makes this distinction in a number of places, though perhaps it's really the latter we want everything to converge upon on given an infinite timescale so perhaps it's reasonable to just expose that. Especially given that we haven't exposed anything for so long.

cc @johannhof

@mikewest
Copy link
Member Author

mikewest commented May 1, 2024

I agree that the latter (relationship with all ancestors) is where things would ideally converge. It's also what we're using today to make decisions about things like SameSite cookies, and explaining that behavior was one of my goals for this header.

I don't object to exposing the relationship to the top-level document, but it's not clear to me what kinds of decisions that would drive for a server that received the header. I would be also interested in @johannhof's take, as he's a lot closer to the partitioning work than I am these days. @arturjanc and @lweichselbaum might have thoughts from Google's server-side perspective as well.

@johannhof
Copy link
Member

I agree that the latter (relationship with all ancestors) is where things would ideally converge. It's also what we're using today to make decisions about things like SameSite cookies, and explaining that behavior was one of my goals for this header.

I think that's also where we want partitioning / third-party cookie blocking to go by default, i.e. roughly the equivalent of SameSite=None being blocked by default, with Storage Access being able to bridge it given permission. Browsers differ in implementations in practice but I do feel like there's good alignment on this goal for security reasons.

The only situation where top-level really makes a difference for web developers then is ABA, right? So, if the developer understood that the resource was cross-site with its ancestors but same-site with its top-level, they might want to utilize storage access got get access to unpartitioned cookies. So I agree this could be useful.

Other related efforts:

  • We're adding a new header to inform sites with storage access permission about their access as part of the Storage Access Headers proposal, which seems compatible with this.
  • We've gotten a lot of feedback around needing a header to understand when a site is in a "partitioned" context, as in third-party cookies / storage are blocked and partitioning is available. It sounds like there's a lot of overlap here so we should probably chat to be able to draw clear lines between this and a potential future proposal :)

CC @DCtheTall @shuranhuang @arichiv @cfredric

@mikewest
Copy link
Member Author

mikewest commented May 2, 2024

We've gotten a lot of feedback around needing a header to understand when a site is in a "partitioned" context, as in third-party cookies / storage are blocked and partitioning is available. It sounds like there's a lot of overlap here so we should probably chat to be able to draw clear lines between this and a potential future proposal :)

I'm confident that things have shifted in the past few years, but I remember talking with @krgovind and @DCtheTall about this a while ago. At that point, I suggested keeping this header the way it's defined here (e.g. processing the whole inclusive ancestor chain), and perhaps introducing some other header (e.g. Sec-Fetch-First-Partyness: ?1 or something).

Assuming those are the axes we actually care about, perhaps we could simplify to a four-state enum? Something like Sec-Fetch-Ancestor-Site with potential values of (top-level (or none?), same-site, same-site-with-top-only, cross-site) would still allow clear explanation of SameSite cookie delivery and allow us to push towards that future boundary, but also distinguish today's partitioning cases in helpful ways?

@arichiv
Copy link
Member

arichiv commented May 2, 2024

same-site-with-top-only is the A>B>A embed case? If so that sounds like good coverage, though I could imagine a frame might want to know if it's cross-site with transient storage, that seems less important.

@annevk
Copy link
Member

annevk commented May 2, 2024

I like the idea of joining the enums. I think we should continue to expose the origin value(s), for websites that want to impose stricter boundaries.

@mikewest
Copy link
Member Author

mikewest commented May 2, 2024

same-site-with-top-only is the A>B>A embed case? If so that sounds like good coverage, though I could imagine a frame might want to know if it's cross-site with transient storage, that seems less important.

Yes, that's the case it's meant to cover.

I like the idea of joining the enums. I think we should continue to expose the origin value(s), for websites that want to impose stricter boundaries.

So: none, same-origin (e.g. A>A>A), same-site (e.g. A>A`>A), same-site-with-top (e.g. A>B>A), cross-site (e.g. A>B)? Or would you want same-origin-with-top-but-only-same-site-with-ancestors too?

@arichiv
Copy link
Member

arichiv commented May 2, 2024

Maybe we could return a list of applicable categories rather than just a single one? Sites that cared about things like origin could look for same-origin (or maybe even a specific category like same-origin-with-top-but-only-same-site-with-ancestors), but others could just check for same-site in the list.

@annevk
Copy link
Member

annevk commented May 2, 2024

Yeah, it seems there's six states (a couple combinations don't make sense which is why it's not nine):

  1. same-origin
  2. same-origin-top-only-but-same-site-otherwise
  3. same-origin-top-only-but-cross-site-otherwise
  4. same-site
  5. same-site-top-only-but-cross-site-otherwise
  6. cross-site

Or maybe:

  1. same-origin
  2. same-site-with-same-origin-top
  3. same-site
  4. cross-site-with-same-origin-top
  5. cross-site-with-same-site-top
  6. cross-site

@mikewest
Copy link
Member Author

mikewest commented May 2, 2024

@annevk: Your latter arrangement seems more conducive to good developer decision-making, but I don't have strong opinions about the naming. I do wonder whether a dictionary would be more appropriate since these really are two semi-independent axes, but being consistent with the rest of fetch metadata probably suggests the single-token approach is the right one.

I think I'd prefer to avoid a list; that feels more difficult for developers to reason about, but I might not be thinking about it the way you are, @arichiv.

@arichiv
Copy link
Member

arichiv commented May 2, 2024

I think @annevk's proposal does cover all existing cases well, my proposal for a list is more about the potential for future expansion. We are designing this based on current partitioning models but if thinking evolved and we had others to add expressing this as a list or a dictionary would be more flexible (servers that didn't care about new tokens in the list or attributes in the dictionary would just ignore them). Otherwise we might be ensuring the need for a new header entirely if partitioning models ever wanted to shift again.

@johannhof
Copy link
Member

I think we're going in the right direction here but don't have anything particularly helpful to add to the bikeshed :)

@mikewest
Copy link
Member Author

mikewest commented May 6, 2024

Thanks all!

I'm unlikely to have a ton of time this week to poke at this PR again to incorporate feedback, but I'm on a plane for too long over the weekend. Should at least be able to solidify the spec then for another round of feedback next week.

@arturjanc
Copy link
Contributor

Thanks @mikewest, this looks broadly reasonable to me. A couple of bikesheds:

  1. WDYT about including an additional value of something like top for top-level navigations and always setting the header? I agree with your point from the PR that this is redundant with Sec-Fetch-Dest: document, but I think that if the header is always present it would make it a bit easier to write server-side logic using the header.
  2. To atone for sending the extra bytes, maybe we could simplify the name to Sec-Fetch-Ancestors? Since we're sending the header also when there's no actual framing (i.e. on subresource requests from the top-level document) it seems like a more general name could fit. Not a hill I'd die on, though :)

@johannhof
Copy link
Member

+1 to Sec-Fetch-Ancestors, this is what my brain autocorrects this proposal's name to anyway

@mozfreddyb
Copy link

With a bit of a delay and apparently my mind elsewhere, I just came to the same conclusion as this thread 😊 . High level: An indication whether a fetch was made as a popup or not would be great.

@johannhof
Copy link
Member

With a bit of a delay and apparently my mind elsewhere, I just came to the same conclusion as this thread 😊 . High level: An indication whether a fetch was made as a popup or not would be great.

Hmm... I might just be missing something but how would you imagine this header to indicate whether a fetch was made from(?) a popup? :)

@annevk
Copy link
Member

annevk commented Nov 4, 2024

Yeah, popups is #83. This is about nested documents only.

@cfredric
Copy link

It seems like we're trying to convey two pieces of info:

  • the relationship between the request's target and all of the frame's ancestors' sites
  • the relationship between the request's target and the top-level frame's site

Maybe it makes sense to convey those in two separate headers, instead of merging them into the same header? If I were writing a server that used these, I think I'd have an easier time using:

  • Sec-Fetch-Ancestors: same-origin, same-site, or cross-site
  • Sec-Fetch-Top-Frame: same-origin, same-site, or cross-site

than the list in #89 (comment).

From a practical standpoint, the two-header version conveys the exact same information as the single-header version; there are 9 possible combos, but as Anne said, some of them are nonsense, so only 6 could actually occur.

However, having just read https://www.mnot.net/blog/2018/11/27/header_compression (found in the Fetch Metadata spec), I think the two-header form would perform a bit better on the wire, since the Sec-Fetch-Top-Frame: ... value would be the same for all fetches under a given top-level document and therefore would likely be found in HPACK's dynamic lookup table more often, IIUC.

@bvandersloot-mozilla
Copy link

Context: @johannhof said @sjledoux is working on these in another thread: whatwg/html#10559 (comment).

What header(s)/values do you want to go forward with?

@sjledoux
Copy link

Hi @bvandersloot-mozilla, for the reasons outlined by @cfredric's last comment in this thread, I was planning on going forward with two headers, Sec-Fetch-Ancestors and Sec-Fetch-Top-Frame, and using the header values of same-origin, same-site, and cross-site for both.

@bvandersloot-mozilla
Copy link

Good thing I bumped into this! I was thinking about prototyping Anne's suggestion because it made a little more sense to me and I didn't see any discussion on Chris's suggestion. However, I'm not going to quibble over which one makes more sense if nobody else has a strong opinion!

Aside on compression: I think the overhead uncompressed is probably more than you get by the increased hit-rate: you've effectively doubled the miss-cost and cache-use, and are only getting a few bytes of benefit where only one is in the cache.

@annevk
Copy link
Member

annevk commented Jan 28, 2025

It'd be great if @arturjanc could solicit some feedback from people who might deploy this on servers.

It's also not clear to me what might lead to more secure deployments. I suspect that with the two header variant we'll have servers with a number of code paths that won't be hit.

And if we go with two headers they should also be named consistently. It should be self-evident they are a pair.

@johannhof
Copy link
Member

Artur is OOO but maybe @ddworken has thoughts on this?

@ddworken
Copy link

I'd also support splitting this into 2 separate headers. When I think about the use cases for this feature, it seems to me like most of them will require looking at either Sec-Fetch-Ancestors or Sec-Fetch-Top-Frame, but not both. For example, one potential security critical use case for this feature could be checking if a request is susceptible to clickjacking. This would only need to involve looking at Sec-Fetch-Ancestors to check the entire ancestor chain. Having to look at and parse the 6-state enum seems like it would involve more work and be more error prone than just checking the 3-state enum. IMO the same is also true of most use cases here.

I also think it is worth noting that this separation of different pieces of info seems broadly similar to how fetch metadata headers are already specced. E.g. Sec-Fetch-User is only delivered for navigation requests, so it could have easily been defined as an extra enum value on Sec-Fetch-Mode (e.g. a user-triggered-navigate). But it was instead split out to a separate header to make usage easier.

@bvandersloot-mozilla
Copy link

Talking to some folks internally, the overhead of sending this seems really worth considering.

I think the use cases for Sec-Fetch-Ancestors are interesting for clickjacking and partitioning clarity, but getting insight from someone who operates servers would be nice. However, since storage partition keying is not top-frame-only anymore, the utility of Sec-Fetch-Top-Frame is not clear to me.

@johannhof
Copy link
Member

@bvandersloot-mozilla

Talking to some folks internally, the overhead of sending this seems really worth considering.

Would be good to understand this in more detail - would you have been fine with a single header but are concerned about two headers? Do you think that the difference is so significant?

I think the use cases for Sec-Fetch-Ancestors are interesting for clickjacking and partitioning clarity, but getting insight from someone who operates servers would be nice.

I think from our perspective @ddworken is the one operating the servers :)

However, since storage partition keying is not top-frame-only anymore, the utility of Sec-Fetch-Top-Frame is not clear to me.

I think the fact that we're having this discussion is evidence that it may have been useful to split them up. As I understand it, SFTF would be the only way for a frame to determine whether they're in an ABA setting or not, no? They could use that information to break out of the partitioning with an autogranted storage access request, as mentioned. Whether this use case warrants a new header is up for debate, but I don't think there's any other way to learn this :)

@bvandersloot-mozilla
Copy link

Would be good to understand this in more detail - would you have been fine with a single header but are concerned about two headers? Do you think that the difference is so significant?

I am just relaying that I've gotten push back internally about header inflation with respect to this.

I think from our perspective @ddworken is the one operating the servers :)

Great! It'd still be nice to have a little more input since the extra bytes aren't opt-in.

I think the fact that we're having this discussion is evidence that it may have been useful to split them up. As I understand it, SFTF would be the only way for a frame to determine whether they're in an ABA setting or not, no? They could use that information to break out of the partitioning with an autogranted storage access request, as mentioned. Whether this use case warrants a new header is up for debate, but I don't think there's any other way to learn this :)

Ah, I meant the utility of the "Site to top level" information isn't clear, not whether it is worth it to split them up. Wouldn't that case be covered by the proposal for Sec-Fetch-Storage-Access: inactive that you're is also advocating? Or just calling document.requestStorageAccess() without user activation? That would conflate the permission-granted state with the ABA state, but I don't think that should really matter to the embed.

@ddworken
Copy link

ddworken commented Feb 3, 2025

I think from our perspective @ddworken is the one operating the servers :)

Great! It'd still be nice to have a little more input since the extra bytes aren't opt-in.

+1. At the very least, during the initial rollouts for other Fetch Metadata headers, we did not experience any issues with the extra request bytes for any Google services. Given this, I'd expect that these two new additions would be similarly unproblematic, especially since they're a smaller addition than the original Fetch Metadata headers.

@sjledoux
Copy link

sjledoux commented Feb 4, 2025

Adding a link to the explainer we just put out for our proposed solution. We are calling this proposal "Frame Ancestor Headers" to refer to both the Sec-Fetch-Top-Frame and Sec-Fetch-Ancestors headers that our approach would involve.

@sjledoux
Copy link

@bvandersloot-mozilla wanted to check if you've had a chance to look over the Frame Ancestor Headers explainer, and if you have any thoughts on it? In particular, wondering whether the additional details it provides on the Sec-Fetch-Storage-Access: inactive use case for Sec-Fetch-Top-Frame helps answer your question about the utility of the Sec-Fetch-Top-Frame, or if that's still unclear in your perspective.

@annevk
Copy link
Member

annevk commented Feb 17, 2025

I still think we should have consistent names if we're going to go with multiple headers instead of one.

@sjledoux
Copy link

I still think we should have consistent names if we're going to go with multiple headers instead of one.

@annevk we're happy to consider alternative header names. Does the pair Sec-Fetch-Frame-Top, and Sec-Fetch-Frame-Ancestors sound better/more consistent?

@annevk
Copy link
Member

annevk commented Feb 21, 2025

Yes, that would be an improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants